Skip to content

time-alarm-service-messages: Make discriminants pub#714

Closed
kurtjd wants to merge 1 commit intoOpenDevicePartnership:v0.2.0from
kurtjd:time-alarm-discriminant
Closed

time-alarm-service-messages: Make discriminants pub#714
kurtjd wants to merge 1 commit intoOpenDevicePartnership:v0.2.0from
kurtjd:time-alarm-discriminant

Conversation

@kurtjd
Copy link
Contributor

@kurtjd kurtjd commented Feb 11, 2026 •
edited
Loading

If we want to deserialize a time-alarm response, we need to know the discriminant:

embedded-services/time-alarm-service-messages/src/lib.rs

Line 292 in 16c4364

fn deserialize(discriminant: u16, buffer: &[u8]) -> Result<Self, MessageSerializationError> {

However, we can't call the trait method discriminant() yet since we don't have a constructed AcpiTimeAlarmResponse. So, make the Discriminant enums public so we can use that to pass a value into deserialize for the response type we are expecting.

This is needed for ec-test-app, e.g.:

impl RtcSource for Serial {
    fn get_capabilities(&self) -> Result<TimeAlarmDeviceCapabilities> {
        let request = AcpiTimeAlarmRequest::GetCapabilities;
        let response = self.send(
            Destination::TimeAlarm,
            request,
            // Need to pass in the expected discriminant to deserialize (which send() calls)
            // If not pub, then we would need to hardcode values here
            AcpiTimeAlarmResponseDiscriminant::Capabilities.into(),
        )?;

        if let AcpiTimeAlarmResponse::Capabilities(capabilities) = response {
            Ok(capabilities)
        } else {
            Err(eyre!("GET_CAPABILITIES received wrong response"))
        }
    }
}

@kurtjd kurtjd requested a review from williampMSFT February 11, 2026 19:30
@kurtjd kurtjd self-assigned this Feb 11, 2026
@kurtjd kurtjd added the enhancement New feature or request label Feb 11, 2026
@kurtjd kurtjd requested a review from a team as a code owner February 11, 2026 19:30
@kurtjd kurtjd moved this to In review in Embedded Controller Feb 11, 2026
@williampMSFT
Copy link
Contributor

williampMSFT commented Feb 11, 2026

The discriminant should come from the MCTP packet header, no? I think I might be missing something

@kurtjd
Copy link
Contributor Author

kurtjd commented Feb 11, 2026

The discriminant should come from the MCTP packet header, no? I think I might be missing something

Yep, playing around with it the discriminant is found within the ODP response header as we discussed offline. So closing this as not necessary. Thanks!

@kurtjd kurtjd closed this Feb 11, 2026
@github-project-automation github-project-automation bot moved this from In review to Done in Embedded Controller Feb 11, 2026
@williampMSFT
Copy link
Contributor

williampMSFT commented Feb 11, 2026

The discriminant should come from the MCTP packet header, no? I think I might be missing something

Yep, playing around with it the discriminant is found within the ODP response header as we discussed offline. So closing this as not necessary. Thanks!

sounds good, filed #715 on myself to clean up some of the header generation macros so they're easier to use from the host side; for now you can probably just manually grab it from the header like you mentioned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@williampMSFT williampMSFT Awaiting requested review from williampMSFT

@felipebalbi felipebalbi Awaiting requested review from felipebalbi felipebalbi is a code owner automatically assigned from OpenDevicePartnership/ec-code-owners

@gjpmsft gjpmsft Awaiting requested review from gjpmsft gjpmsft is a code owner automatically assigned from OpenDevicePartnership/ec-code-owners

@madeleyneVaca madeleyneVaca Awaiting requested review from madeleyneVaca madeleyneVaca is a code owner automatically assigned from OpenDevicePartnership/ec-code-owners

Assignees

@kurtjd kurtjd

Labels

enhancement New feature or request

Projects

Embedded Controller
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kurtjd @williampMSFT